[fuzz] fix minor fuzz bugs#14593
Conversation
jmarantz
left a comment
jmarantz
left a comment
There was a problem hiding this comment.
just a couple of nits/clarifications
| NiceMock<Server::Configuration::MockFactoryContext> factory_context; | ||
| EXPECT_THROW_WITH_MESSAGE(admission_control_filter_factory.createFilterFactoryFromProtoTyped( | ||
| proto, "whatever", factory_context), | ||
| EnvoyException, "Success Rate Threshold cannot be less than one."); |
There was a problem hiding this comment.
ah I see here that we are just worried about it being a small positive float. OK, then I'd write "less than 1.0".
There was a problem hiding this comment.
Yep! A small positive float can eventually round down to a division by zero.
The alternative is to round up in this division but I don't want to change behavior for reasonable values (above 1) OR take the max of 1 and this value.
There was a problem hiding this comment.
Just looping @tonya11en @mattklein123
The bounds for threshold described here say [0, 1]. https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/admission_control_filter
Should it actually be (0, 1] to avoid undefined behavior -- and in practice should it be [0.01, 1.0] (which is what this fix provides, blocking success rate threshold below 1.0%?)
There was a problem hiding this comment.
Yeah I think this is reasonable. 👍
Signed-off-by: Asra Ali <asraa@google.com>
* master: http: support creating filters with match tree (envoyproxy#14430) [tls] Expose ServerContextImpl::selectTlsContext (envoyproxy#14592) docs: update ext_proc docs to reflect implementation status (envoyproxy#14636) filter manager: drop assert (envoyproxy#14633) kick off v1.18.0 (envoyproxy#14637) 1.17.0 release (envoyproxy#14624) Implement request header processing in ext_proc (envoyproxy#14385) http: expose encoded headers/trailers via callbacks (envoyproxy#14544) [fuzz] fix minor fuzz bugs (envoyproxy#14593) rate limit: add computed descriptors (envoyproxy#14448) tools: fill in the required args for Api::Impl (envoyproxy#14554) Bump envoy-build to current images (envoyproxy#14608) Signed-off-by: Michael Puncel <mpuncel@squareup.com>
3 participants
Signed-off-by: Asra Ali asraa@google.com
Commit Message: Fixes minor fuzz bugs.
Risk Level: Low
Testing: Added unit test for admission control fix
Fixes
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28401
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28696
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29233